Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy #5962

seanlaii · 2024-12-19T00:22:58Z

What type of PR is this?
/kind api-change
/kind feature

What this PR does / why we need it:
Add SchedulePriority to PropagationPolicy & ClusterPropagationPolicy to support flexible priority class resolution strategies:

KubePriorityClass: Use Kubernetes native PriorityClass
PodPriorityClass: Use priority from workload's PodTemplate
FederatedPriorityClass: Reserved for future Karmada-specific priority classes

This enhancement allows users to:

Specify which priority class source to use
Control resource scheduling priority
Maintain consistent priority behavior with Kubernetes

Which issue(s) this PR fixes:
Part of #5961

Special notes for your reviewer:

Default source is KubePriorityClass
Falls back to global default if unspecified
FederatedPriorityClass is reserved for future implementation
All new fields are optional to maintain backward compatibility

Does this PR introduce a user-facing change?:

Add SchedulePriority field to PropagationPolicy and ClusterPropagationPolicy:
- Supports multiple priority class sources (Kubernetes, PodTemplate, Federated[future])
- Configurable priority class resolution strategy
- Maintains compatibility with Kubernetes priority behavior
- Optional fields preserve backward compatibility

codecov-commenter · 2024-12-19T00:54:13Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.10%. Comparing base (f555413) to head (4bf165a).
Report is 4 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5962      +/-   ##
==========================================
- Coverage   48.12%   48.10%   -0.02%     
==========================================
  Files         668      668              
  Lines       55291    55291              
==========================================
- Hits        26607    26599       -8     
- Misses      26949    26956       +7     
- Partials     1735     1736       +1

Flag	Coverage Δ
unittests	`48.10% <ø> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

RainbowMango

/assign
Start working on it!

RainbowMango

@whitewindmills Would you like to take another look?

RainbowMango · 2025-02-18T02:44:23Z

pkg/apis/policy/v1alpha1/propagation_types.go

+	// SchedulePriority defines how Karmada should resolve the priority and preemption policy
+	// for workload scheduling.
+	//
+	// Valid options for PriorityClassSource are:
+	// - KubePriorityClass (default): Uses Kubernetes PriorityClass (scheduling.k8s.io/v1)
+	// - PodPriorityClass: Uses PriorityClassName from PodTemplate (PodSpec.PriorityClassName)
+	// - FederatedPriorityClass: Uses Karmada FederatedPriorityClass (not yet implemented)
+	//
+	// PriorityClassName behavior:
+	//
+	// For KubePriorityClass:
+	// - If specified: Uses the named Kubernetes PriorityClass
+	// - If empty: Uses cluster's global default PriorityClass
+	// - If neither exists: Sets priority=0 and preemptionPolicy=Never
+	//
+	// For PodPriorityClass:
+	// - Uses PriorityClassName from PodTemplate
+	// - Falls back to global default if not found
+	// - If no valid PriorityClass found: Sets priority=0 and preemptionPolicy=Never
+	//
+	// For FederatedPriorityClass:
+	// - Reserved for future use
+	// - Current behavior undefined
+	//
+	// +optional
+	SchedulePriority *SchedulePriority `json:"schedulePriority,omitempty"`


Maybe we don't have to explain the valid options outside, because they are described inside of SchedulePriority. But we can give some overall descriptions, like:

Suggested change

// SchedulePriority defines how Karmada should resolve the priority and preemption policy

// for workload scheduling.

//

// Valid options for PriorityClassSource are:

// - KubePriorityClass (default): Uses Kubernetes PriorityClass (scheduling.k8s.io/v1)

// - PodPriorityClass: Uses PriorityClassName from PodTemplate (PodSpec.PriorityClassName)

// - FederatedPriorityClass: Uses Karmada FederatedPriorityClass (not yet implemented)

//

// PriorityClassName behavior:

//

// For KubePriorityClass:

// - If specified: Uses the named Kubernetes PriorityClass

// - If empty: Uses cluster's global default PriorityClass

// - If neither exists: Sets priority=0 and preemptionPolicy=Never

//

// For PodPriorityClass:

// - Uses PriorityClassName from PodTemplate

// - Falls back to global default if not found

// - If no valid PriorityClass found: Sets priority=0 and preemptionPolicy=Never

//

// For FederatedPriorityClass:

// - Reserved for future use

// - Current behavior undefined

//

// +optional

SchedulePriority *SchedulePriority `json:"schedulePriority,omitempty"`

// SchedulePriority defines how Karmada should resolve the priority and preemption policy

// for workload scheduling.

//

// This setting is particularly useful for controlling the scheduling behavior of offline workloads.

// By setting a higher or lower priority, users can control which workloads are scheduled first.

// Additionally, it also allows to specify the preempt policy that higher-priority workloads can

// preempt lower-priority ones in scenarios where resource contention occurs.

//

// Note: This feature is currently in the alpha stage. The priority-based scheduling functionality is

// controlled by the PriorityBasedScheduling feature gate, and preemption is controlled by the

// PriorityBasedPreemptiveScheduling feature gate. At this time, only priority-based scheduling is

// supported, while preemption functionality is not yet available and will be introduced in the future

// releases as the feature matures.

//

// +optional

SchedulePriority *SchedulePriority `json:"schedulePriority,omitempty"`

Thanks, updated.

RainbowMango · 2025-02-18T02:47:53Z

pkg/apis/policy/v1alpha1/propagation_types.go

+	// - KubePriorityClass (default): Uses Kubernetes PriorityClass (scheduling.k8s.io/v1)
+	// - PodPriorityClass: Uses PriorityClassName from PodTemplate (PodSpec.PriorityClassName)
+	// - FederatedPriorityClass: Uses Karmada FederatedPriorityClass (not yet implemented)
+	//
+	// +kubebuilder:default="KubePriorityClass"
+	// +kubebuilder:validation:Enum=FederatedPriorityClass;KubePriorityClass;PodPriorityClass
+	// +optional


Currently, we provide support for KubePriorityClass only, right?

Shall we add a comment to PodPriorityClass to explain a little bit, something like not yet implemented?

In addition, please shorten the +kubebuilder:validation:Enum with supported options.

RainbowMango · 2025-02-18T02:55:13Z

pkg/apis/policy/v1alpha1/propagation_types.go

+	// - Reserved for future use
+	// - Current behavior undefined
+	//
+	// +optional


Suggested change

// +optional

// +required

Can we restrict this as a required in the current version, just hope to work out a simple version quickly.

@seanlaii In your use cases, will you set a specific PriorityClassName for each workload, or will you rely on the default one?

Sure, we currently set a default priority class for every workload if no priority class is set.

Changed it to required.

Sure, we currently set a default priority class for every workload if no priority class is set.

+1
I think that making the configuration clearer, and also a relief for the implementation.

whitewindmills · 2025-02-18T07:11:21Z

/assign

whitewindmills · 2025-02-18T07:15:49Z

pkg/apis/policy/v1alpha1/propagation_types.go

+	//
+	// For PodPriorityClass:
+	// - Uses PriorityClassName from PodTemplate
+	// - Falls back to global default if not found


I'm afraid people don't know what the global default is.

I changed the description to

// If the specified PriorityClass is not found, falls back to the cluster's default PriorityClass // (i.e., the PriorityClass marked as the global default in the cluster).

WDYT?

Signed-off-by: wei-chenglai <[email protected]>

RainbowMango

/lgtm
/approve

I'm going to move this forward to avoid blocking the other tasks.
And I'll double check the comments before releasing, ensuring the comments consist with the minimal version.

karmada-bot · 2025-02-18T16:17:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RainbowMango]
~~pkg/apis/OWNERS~~ [RainbowMango]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

karmada-bot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Dec 19, 2024

karmada-bot requested a review from chaosi-zju December 19, 2024 00:23

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 19, 2024

karmada-bot requested review from Poor12 and XiShanYongYe-Chang December 19, 2024 00:23

karmada-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 19, 2024

RainbowMango mentioned this pull request Dec 19, 2024

[Feature] [Alpha] Application Priority and Preemption #5961

Open

11 tasks

seanlaii force-pushed the priorityclass branch from 2d81d50 to 25e0f40 Compare January 19, 2025 00:25

karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 19, 2025

RainbowMango reviewed Feb 18, 2025

View reviewed changes

karmada-bot assigned RainbowMango Feb 18, 2025

RainbowMango reviewed Feb 18, 2025

View reviewed changes

karmada-bot assigned whitewindmills Feb 18, 2025

whitewindmills reviewed Feb 18, 2025

View reviewed changes

Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy

4bf165a

Signed-off-by: wei-chenglai <[email protected]>

seanlaii force-pushed the priorityclass branch from 25e0f40 to 4bf165a Compare February 18, 2025 12:23

RainbowMango approved these changes Feb 18, 2025

View reviewed changes

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2025

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 18, 2025

karmada-bot merged commit 9f8d71e into karmada-io:master Feb 18, 2025
21 checks passed

seanlaii deleted the priorityclass branch February 18, 2025 16:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy #5962

Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy #5962

seanlaii commented Dec 19, 2024 •

edited

Loading

codecov-commenter commented Dec 19, 2024 •

edited

Loading

RainbowMango left a comment

RainbowMango left a comment

RainbowMango Feb 18, 2025

seanlaii Feb 18, 2025

RainbowMango Feb 18, 2025

RainbowMango Feb 18, 2025

seanlaii Feb 18, 2025

seanlaii Feb 18, 2025

RainbowMango Feb 18, 2025

whitewindmills commented Feb 18, 2025

whitewindmills Feb 18, 2025

seanlaii Feb 18, 2025

RainbowMango left a comment

karmada-bot commented Feb 18, 2025

Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy #5962

Add priorityClassName in PropagationPolicy & ClusterPropagationPolicy #5962

Conversation

seanlaii commented Dec 19, 2024 • edited Loading

codecov-commenter commented Dec 19, 2024 • edited Loading

Codecov Report

RainbowMango left a comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whitewindmills commented Feb 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Feb 18, 2025

seanlaii commented Dec 19, 2024 •

edited

Loading

codecov-commenter commented Dec 19, 2024 •

edited

Loading